fix: add mandatory SHA-256 checksum verification to install.sh#226
Open
vinayada1 wants to merge 3 commits intoprivateerproj:mainfrom
Open
fix: add mandatory SHA-256 checksum verification to install.sh#226vinayada1 wants to merge 3 commits intoprivateerproj:mainfrom
vinayada1 wants to merge 3 commits intoprivateerproj:mainfrom
Conversation
The installer previously piped the downloaded archive directly into tar without any integrity check, allowing a compromised release asset or MITM to silently deliver a tampered binary. Changes: - Download the release archive to a temp file before extraction - Fetch the GoReleaser checksums.txt asset from the same release - Verify SHA-256 of the archive against the checksums file before extracting; abort on mismatch, missing asset, or download failure (fail-closed — no fallback to unverified install) - Extract asset URLs with a structured parser (extract_download_urls / find_release_asset_url) that works on both compact and multiline GitHub API JSON, replacing brittle grep|cut scraping - Guard main() behind BASH_SOURCE so the script can be safely sourced in tests without triggering installation - Wire installer tests into make test via Makefile - Add .PHONY declarations to prevent make skipping targets when a directory named 'test' exists Tests added (test/install_test.sh): - Happy path with multiline JSON response - Happy path with compact single-line JSON response - Fail when checksums asset is absent from release - Fail when checksums file download errors - Fail when SHA-256 digest mismatches
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the install.sh installer by making release downloads verify a mandatory SHA-256 checksum before extracting and installing the pvtr binary, reducing supply-chain risk for end users.
Changes:
- Download release archive to a temp directory, fetch
checksums.txt, and verify SHA-256 before extraction. - Replace fragile URL parsing with
extract_download_urls/find_release_asset_urlhelpers that work across compact/multiline GitHub API JSON. - Add a dedicated Bash test (
test/install_test.sh) and run it as part ofmake test; add aBASH_SOURCEguard soinstall.shcan be sourced in tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
install.sh |
Adds checksum enforcement and new release-asset URL extraction helpers; makes script safe to source in tests. |
test/install_test.sh |
Adds mocked integration-style tests covering checksum success/failure and JSON parsing formats. |
Makefile |
Runs the new installer test during the test target and declares phony targets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jmeridth
added a commit
that referenced
this pull request
Apr 8, 2026
## What Extracted the integration test job from ci.yml into a dedicated integration.yml workflow and replaced octo-sts OIDC token exchange with the default GITHUB_TOKEN in both integration.yml and osps-security-assessment.yml. Deleted the STS policy file. ## Why Both workflows only perform read-only operations on public repos. The default GITHUB_TOKEN is sufficient and available to fork PRs under the pull_request event, eliminating the OIDC minting that caused fork PR failures (e.g. PRs #226, #227). ## Notes - Deleted .github/chainguard/privateer.sts.yaml since no workflow in this repo uses octo-sts anymore - The release.yml workflow still uses octo-sts for homebrew-tap access, but its STS policy lives in that repo - Removed id-token:write from both workflows, reducing privilege scope - Added explicit permissions: contents: read to the changes job in integration.yml Signed-off-by: jmeridth <[email protected]>
jmeridth
added a commit
that referenced
this pull request
Apr 8, 2026
## What Extracted the integration test job from ci.yml into a dedicated integration.yml workflow and replaced octo-sts OIDC token exchange with the default GITHUB_TOKEN in both integration.yml and osps-security-assessment.yml. Deleted the STS policy file. ## Why Both workflows only perform read-only operations on public repos. The default GITHUB_TOKEN is sufficient and available to fork PRs under the pull_request event, eliminating the OIDC minting that caused fork PR failures (e.g. PRs #226, #227). ## Notes - Deleted .github/chainguard/privateer.sts.yaml since no workflow in this repo uses octo-sts anymore - The release.yml workflow still uses octo-sts for homebrew-tap access, but its STS policy lives in that repo - Removed id-token:write from both workflows, reducing privilege scope - Added explicit permissions: contents: read to the changes job in integration.yml Signed-off-by: jmeridth <[email protected]>
Member
|
@vinayada1 This change is good if we hadn't started work on #204. If we can finish that work, we can use an actual pvtr command and not bash scripts. I'm willing to approve and merge this for now but that PR needs to be a quick follow. Can you please respond to the Copilot feedback. |
Contributor
|
Yes, actually my headspace on the other PR installer was primarily focused on plugins — so this is perfect. |
Signed-off-by: Vinaya Damle <[email protected]>
vinayada1
force-pushed
the
fix/installer-checksum-verification
branch
from
17ac43c to
7f3149e
Compare
Contributor
Author
Addressed the comments |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
install.shpreviously downloaded the release archive and piped it directly intotarwith no integrity verification. A supply-chain compromise, swapped release asset, or MITM could deliver a tampered binary without any indication. This change makes verification mandatory and fail-closed:checksums.txtasset is fetched from the same releaseextract_download_urls/find_release_asset_urlhelper that works on both compact and multiline GitHub API JSON, replacing the previous fragilegrep|cutapproachBASH_SOURCEguard added so the script can be safely sourced in tests without triggering installationBreaking changes
install.shwill now refuse to install if a release does not include achecksums.txtasset. All published releases using GoReleaser include this file by default. Pre-GoReleaser releases without the asset will no longer install via this script.